[4기 - 전선희] SpringBoot Part3 Weekly Mission 첫 번째 PR 제출합니다! by funnysunny08 · Pull Request #858 · prgrms-be-devcourse/springboot-basic

funnysunny08 · 2023-07-23T14:42:06Z

📌 과제 설명

6주차 강의에서 배운 내용들을 활용하여 바우처 관리 페이지 및 기능 페이지, API를 만들었습니다!!

👩‍💻 요구 사항과 구현 내용

커맨드로 지원했던 기능을 thymeleaf를 이용해서 관리페이지를 만들고 다음 기능 구현
- 조회페이지
- 상세페이지
- 입력페이지
- 삭제기능
Spring MVC를 적용해서 JSON과 XML을 지원하는 REST API 개발
- 전체 조회기능
- 조건별 조회기능 (바우처 생성기간 및 특정 할인타입별)
- 바우처 추가기능
- 바우처 삭제기능
- 바우처 아이디로 조회 기능

✅ PR 포인트 & 궁금한 점

JoosungKwon

선희님 3주차 과제까지 정말 고생많으셨습니다!!! 👏👏👏

제가 실행 시켜봤는데 기본 데이터나 스키마 SQL이 없어서 테스트 해볼 수 없었네요.!! 실행할 수 있도록 추가해주면 더욱 자세하게 확인할 수 있을 것 같아요! 그리고 진입할 수 있는 인덱스 페이지가 있었으면 좋을 것 같네요. 😄
데브코스 과제를 단순히 제출을 위한 과제로 생각안하셨으면 좋겠습니다! 제출만 하는 것은 사실 큰 의미가 없습니다. 테스트 코드도 많이 작성해보시고, 다이어그램도 그려보고, 리드미에 내용도 추가하는 등 다양한 시도도 해보시면서 하나의 작은 프로그램 혹은 서비스를 만든다고 생각해보시면서 완성도를 높이면 좋을 것 같습니다!! 👍 (강요는 아닙니다~~!! JPA과제가 바쁘면 먼저 JPA 과제를 하는게 맞습니다.)

JoosungKwon · 2023-07-25T09:39:23Z

src/main/resources/application.properties

+spring.datasource.username=root
+spring.datasource.password=0828


보안 정보나 개인 정보를 깃허브에 올리시면 안됩니다.. 😢

[4기 - 전선희] SpringBoot Part2 Weekly Mission 첫 번째 PR 제출합니다! #764 (comment)

JoosungKwon · 2023-07-25T09:42:39Z

src/main/resources/logFiles/log.2023-07-06.log

@@ -0,0 +1,89 @@
+15:20:48.186 [main] ERROR c.p.s.r.v.JdbcVoucherRepository - Got empty result


log 파일도 .gitignore에 추가해서 안올라오도록 하시면 좋을 것 같아요!

[4기 - 전선희] SpringBoot Part2 Weekly Mission 첫 번째 PR 제출합니다! #764 (comment)

JoosungKwon · 2023-07-25T10:54:10Z

src/test/java/com/prgrms/spring/VoucherRepositoryTest.java

+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
+
+@SpringJUnitConfig


@JdbcTest에 대해서 알아보셔도 좋을 것 같아요!

JoosungKwon · 2023-07-25T10:57:24Z

src/test/java/com/prgrms/spring/VoucherRepositoryTest.java

+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)


테스트 코드를 하나의 라이프사이클로 관리하시는 이유가 궁금합니다~

JoosungKwon · 2023-07-25T11:20:50Z

src/main/java/com/prgrms/spring/common/advice/ControllerExceptionAdvice.java

+    protected ApiResponse<Object> handleException(final Exception error, final HttpServletRequest request) throws IOException {
+        return ApiResponse.error(Error.INTERNAL_SERVER_ERROR);
+    }
+
+    /**
+     * custom error
+     */
+    @ExceptionHandler(CustomException.class)
+    protected ResponseEntity<ApiResponse> handleSoptException(CustomException e) {
+        return ResponseEntity.status(e.getHttpStatus())
+                .body(ApiResponse.error(e.getError(), e.getMessage()));


CustomException은 final이 아니네요.. 선희님은 handleException의 매개변수를 왜 final로 하셨나요? 그리고 이유가 있다면 통일성을 지키면 좋을 것 같아요! 달라야만 하는 이유가 있나요?

JoosungKwon · 2023-07-25T11:52:38Z

src/main/java/com/prgrms/spring/domain/voucher/FixedAmountVoucher.java

+
+@AllArgsConstructor(access = AccessLevel.PRIVATE)
+public class FixedAmountVoucher implements Voucher {
+    private final String voucherName = "FixedAmountVoucher";


VoucherType이 있는데 voucherName을 따로 두신 이유가 궁금합니다!

JoosungKwon · 2023-07-25T11:54:40Z

src/main/java/com/prgrms/spring/domain/voucher/PercentDiscountVoucher.java

+    private final String voucherName = "PercentDiscountVoucher";
+    private final UUID voucherId;
+    private final long discount;
+    private final String discountUnit = "%";


선희님은 어떤 필드에 static을 사용하는게 적합하다고 생각하시나요?

JoosungKwon · 2023-07-25T14:58:00Z

src/main/java/com/prgrms/spring/service/VoucherService.java

+        switch (voucherType) {
+            case PERCENT_DISCOUNT:
+                discountUnit = "%";
+                break;
+            case FIXED_AMOUNT:
+                discountUnit = "$";
+                break;
+        }


discountUnit을 voucherType의 필드로 두면 분기문이 필요 없을 것 같아요!

JoosungKwon · 2023-07-25T15:04:02Z

src/main/java/com/prgrms/spring/service/VoucherService.java

+    @Transactional
+    public VoucherResponseDto createVoucher(VoucherCreateRequestDto requestDto) {
+        Voucher voucher = null;
+        if (VoucherType.valueOf(requestDto.getVoucherType()) == VoucherType.FIXED_AMOUNT) {


DTO에서 VoucherType을 가져오는 것은 어떤가요?

JoosungKwon · 2023-07-25T15:06:16Z

src/main/java/com/prgrms/spring/service/VoucherService.java

+
+    @Transactional
+    public void deleteVoucher(String voucherId) {
+        Voucher voucher = voucherRepository.findById(UUID.fromString(voucherId))


삭제할 데이터가 없으면 예외상황인가요? 선희님은 어떻게 생각하시나요~?

kakao-gray-great

안녕하세요 선희님 :)

코드 너무 잘 짜주셨네요 💯
코드가 깔끔하게 짜져서 있어서 읽는데 굉장히 편했습니다 👍

주성님이 남겨주신 피드백 외에 크게 리뷰드릴게 많이 없어서 간단하게 몇 개 남겼으니 확인 부탁드려요~
추가로 Repository에 대한 테스트 코드만 작성하셨는데 이유가 있으신지 궁금합니다ㅎ

kakao-gray-great · 2023-07-28T01:43:54Z

src/main/java/com/prgrms/spring/controller/api/VoucherRestController.java

+    @GetMapping("/{voucherId}")
+    @ResponseStatus(HttpStatus.OK)
+    public ApiResponse<VoucherResponseDto> getVoucherById(@PathVariable String voucherId) {
+        return ApiResponse.success(Success.GET_VOUCHER_SUCCESS, voucherService.getVoucherById(voucherId));


모든걸 한 줄에 표현하는건 가독성이 좋지 않은 문제가 있어요.
저라면 id로 값을 꺼낸 변수를 success 인자로 넣어줄 것 같아요.
그게 더 눈에 잘 들어오지 않을까요?

kakao-gray-great · 2023-07-28T01:46:04Z

src/main/java/com/prgrms/spring/controller/dto/request/CustomerCreateRequestDto.java

+import lombok.Getter;
+import lombok.NoArgsConstructor;
+
+@Getter


dto는 data class여서 @Getter를 사용하실거면 @DaTa 를 사용하시는 것도 고려해보시면 좋을 것 같아요

kakao-gray-great · 2023-07-28T01:46:53Z

src/main/java/com/prgrms/spring/domain/voucher/FixedAmountVoucher.java

+    }
+
+    private static void chkDiscountValue(long discount) {
+        if (discount <= 0) throw new IllegalArgumentException("유효하지 않은 할인 금액");


한 줄에 해주는 것도 좋지만, 중괄호가 있는 것이 전체적인 통일성 측면에서 좋을 것 같아요.

kakao-gray-great · 2023-07-28T01:48:23Z

src/main/java/com/prgrms/spring/service/VoucherService.java

+        if (VoucherType.valueOf(requestDto.getVoucherType()) == VoucherType.FIXED_AMOUNT) {
+            voucher = FixedAmountVoucher.newInstance(UUID.randomUUID(), requestDto.getDiscount());
+        } else if (VoucherType.valueOf(requestDto.getVoucherType()) == VoucherType.PERCENT_DISCOUNT) {
+            voucher = PercentDiscountVoucher.newInstance(UUID.randomUUID(), requestDto.getDiscount());
+        }


else if를 안쓰고 어떻게 바꿔볼 수 있을까요?

else if를 쓰지말라고 하는 이유가 많은데 왜 인지 아실까요?

funnysunny08 added 30 commits June 23, 2023 19:02

FEAT : domain, controller, repository, service단 완료

70818f1

FEAT : create AppRunner

521936d

FIX : RequiredArgsConstructor -> AllArgsConstructor로 변경

9ca33e2

REFACTOR : 패키지명 변경

f3d3d73

REFACTOR : 코드 정렬

d155237

REFACTOR : final 키워드 추가

a23b63e

REFACTOR : 불필요한 파일 제거 (AppConfiguration)

7c30219

FEAT : CommandLineRunner 적용

6b7440a

FEAT : storage 반환 시 방어적 복사 적용

173a5b0

FEAT : 입출력 분리, ConsoleView 생성

1d20a53

FEAT : 타입 매칭 예외 처리 메시지

db596fc

FEAT : ~Voucher에 voucherName, voucherUnit 필드 추가

90f6b4d

FEAT : logger 추가

bcbbdf3

REFACTOR : VoucherService 데이터 가공 -> controller 단으로

ff48099

FEAT : input 예외 처리 추가

50e3723

FEAT : discount 값 범위 검증

ce5bcee

[FEAT] JdbcVoucherRepository

fd00f7c

[Feat] dependency 수정

12a61e5

[FEAT] Customer Entity

39b9134

[FEAT] Customer Repository (with jdbc)

c3a62d8

[Feat] Customer Service

595b3e2

[Feat] JdbcVoucherRepository test

066f712

[Feat] Customer Controller

ebacd64

[Feat] Transcational readonly 추가

2af5384

[Refactor] 전체 조회 string 변환 부분 service단으로

cd563da

[Feat] custom 예외

a8e2284

[Feat] create customer controller에서 input 분리

2d268d1

[Feat] 고객 생성 dto 생성을 consoleView로

2cb01b9

[Refacotr]

a6bf4d1

[Refactor] discount 값 검증 메소드로 분리:

92cd9d1

funnysunny08 added 10 commits July 13, 2023 14:33

[Refactor] 테스트에서 로그 삭졔

c089618

[Feat] 전체 조회 기능 api

e509d56

[Feat] 조건별 조회 기능 API

1468bb6

[Feat] 바우처 추가 API

0b9187f

[Feat] 바우처 삭제 API

16b8f5a

[Feat] 바우처 아이디로 조회 API

9bc8880

[Feat] api 완성

e95e709

[Feat] view 관련 api, view 작성

52f9066

[Refactor] 불필요한 파일 제거

2e83dc5

[Refactor] 불필요한 코드 삭제

5fcb8eb

funnysunny08 added 🔥 매운맛 Team-Navy labels Jul 23, 2023

funnysunny08 requested review from JoosungKwon and kakao-gray-great July 23, 2023 14:42

funnysunny08 self-assigned this Jul 23, 2023

JoosungKwon reviewed Jul 25, 2023

View reviewed changes

kakao-gray-great reviewed Jul 28, 2023

View reviewed changes

		spring.datasource.username=root
		spring.datasource.password=0828

		@@ -0,0 +1,89 @@
		15:20:48.186 [main] ERROR c.p.s.r.v.JdbcVoucherRepository - Got empty result

		@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
		@TestInstance(TestInstance.Lifecycle.PER_CLASS)

Conversation

funnysunny08 commented Jul 23, 2023

📌 과제 설명

👩‍💻 요구 사항과 구현 내용

✅ PR 포인트 & 궁금한 점

Uh oh!

JoosungKwon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kakao-gray-great left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JoosungKwon left a comment •

edited

Loading